fix: avoid ListView take_reduce rebuild for dense selections by dimitarvdimitrov · Pull Request #7339 · vortex-data/vortex

dimitarvdimitrov · 2026-04-08T13:25:11Z

The take_reduce path rebuilds the ListView, which is expensive.

For dense selections (density above the rebuild threshold) there's little garbage to reclaim, so skip the rebuild and let the regular take path handle it. The rebuild is still worthwhile for sparse takes where we'd otherwise drag around a lot of unused elements when exporting (e.g. to DuckDB).

This PR does a very crude estimation of the number of elements that stay in the ListViewArray.

Potentially we should do this estimation at export time in the ListViewExporter, but that's less trivial.

codspeed-hq · 2026-04-08T16:43:32Z

Merging this PR will improve performance by 20.23%

⚡ 9 improved benchmarks
✅ 1154 untouched benchmarks
⏩ 1457 skipped benchmarks¹

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
⚡	Simulation	`take_map[(0.1, 0.5)]`	1,154.5 µs	970.7 µs	+18.94%
⚡	Simulation	`take_map[(0.1, 1.0)]`	2 ms	1.6 ms	+20.23%
⚡	Simulation	`patched_take_10k_contiguous_not_patches`	258.4 µs	228.2 µs	+13.25%
⚡	Simulation	`patched_take_10k_contiguous_patches`	258.1 µs	227.8 µs	+13.27%
⚡	Simulation	`patched_take_10k_dispersed`	316 µs	285.8 µs	+10.56%
⚡	Simulation	`patched_take_10k_first_chunk_only`	302 µs	271.7 µs	+11.15%
⚡	Simulation	`patched_take_10k_random`	270.3 µs	240 µs	+12.63%
⚡	Simulation	`take_10k_dispersed`	284.4 µs	239.5 µs	+18.75%
⚡	Simulation	`take_10k_first_chunk_only`	270.6 µs	225.6 µs	+19.91%

_{Comparing mitko/avoid-list-view-take-reduce-rebuild (2eb3db2) with develop (01679cd)}

1457 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

gatesn · 2026-04-08T18:24:28Z

Am I missing something? Why would we ever rebuild? The whole point of ListView is to have cheap take/filter.

It should be explicit from the user if they want to garbage collect

dimitarvdimitrov · 2026-04-08T18:48:22Z

Am I missing something? Why would we ever rebuild? The whole point of ListView is to have cheap take/filter.

It should be explicit from the user if they want to garbage collect

The rebuild is still worthwhile for sparse takes where we'd otherwise drag around a lot of unused elements when exporting e.g. duckdb's ListViewExporter

vortex/vortex-duckdb/src/exporter/list_view.rs

Lines 80 to 85 in 11d607e

    
           let elements_exporter = 
        
               new_array_exporter_with_flatten(elements.clone(), cache, ctx, true)?; 
        
           if !elements.is_empty() { 
        
               elements_exporter.export(0, elements.len(), &mut duckdb_elements, ctx)?; 
        
           }

ideally we do this as heuristics in the exporter as opposed to in each operation like take. I was thinking of doing that next, but it might take a bit longer

gatesn · 2026-04-08T19:03:43Z

ideally we do this as heuristics in the exporter as opposed to in each operation like take

Yeah, 100%. Agree priority might not be to do it now, but eager GC impacts all Vortex users

When the selection density is above the threshold, skip take_reduce so we don't rebuild the ListView for dense takes. Signed-off-by: Dimitar Dimitrov <mitko@spiraldb.com> Signed-off-by: Dimitar Dimitrov <dimitar@spiraldb.com>

dimitarvdimitrov · 2026-04-09T12:33:26Z

looking into why the benchmarks regressed...

Signed-off-by: Dimitar Dimitrov <dimitar@spiraldb.com>

joseph-isaacs · 2026-04-10T14:31:11Z

+// TODO(connor)[ListView]: Ideally, we would only rebuild after all `take`s and `filter`
+//  compute functions have run, at the "top" of the operator tree. However, we cannot do this
+//  right now, so we will just rebuild every time (similar to [`ListArray`]).
+pub const REBUILD_DENSITY_THRESHOLD: f32 = 0.1;


i think this should move? and also be pub crate?

any suggestion for where to move it? it's now used in arrays/listview/compute and arrays/filter/execute

Signed-off-by: Dimitar Dimitrov <dimitar@spiraldb.com>

…iew-take-reduce-rebuild

joseph-isaacs · 2026-04-16T15:32:15Z

+// TODO(connor)[ListView]: Ideally, we would only rebuild after all `take`s and `filter`
+//  compute functions have run, at the "top" of the operator tree. However, we cannot do this
+//  right now, so we will just rebuild every time (similar to [`ListArray`]).
+pub(crate) const REBUILD_DENSITY_THRESHOLD: f32 = 0.1;


why does this live here and not in the compute mod still as pub(crate)?

joseph-isaacs

Fix this and this is ready to go

github-actions · 2026-04-16T15:34:54Z

BENCHMARK FAILED

Benchmark Compression failed! Check the workflow run for details.

github-actions · 2026-04-16T15:34:55Z

BENCHMARK FAILED

Benchmark Random Access failed! Check the workflow run for details.

github-actions · 2026-04-16T15:41:46Z

Polar Signals Profiling Results

Latest Run

Status	Commit	Job	Attempt	Link
🟡 In Progress	`df8bb7d`		1	Explore Profiling Data

Powered by Polar Signals Cloud

Signed-off-by: Dimitar Dimitrov <dimitar@spiraldb.com>

joseph-isaacs reviewed Apr 8, 2026

View reviewed changes

Comment thread vortex-array/src/arrays/listview/compute/take.rs Outdated

joseph-isaacs reviewed Apr 8, 2026

View reviewed changes

Comment thread vortex-array/src/arrays/listview/compute/take.rs Outdated

joseph-isaacs reviewed Apr 8, 2026

View reviewed changes

Comment thread vortex-array/src/arrays/listview/compute/take.rs Outdated

joseph-isaacs reviewed Apr 8, 2026

View reviewed changes

Comment thread vortex-array/src/arrays/listview/compute/take.rs Outdated

joseph-isaacs reviewed Apr 8, 2026

View reviewed changes

Comment thread vortex-array/src/arrays/listview/vtable/mod.rs Outdated

dimitarvdimitrov force-pushed the mitko/avoid-list-view-take-reduce-rebuild branch from 1074601 to 0cd4f36 Compare April 9, 2026 11:23

dimitarvdimitrov added the changelog/skip Do not list PR in the changelog label Apr 9, 2026

Merge branch 'develop' into mitko/avoid-list-view-take-reduce-rebuild

6daa380

dimitarvdimitrov marked this pull request as ready for review April 9, 2026 15:54

joseph-isaacs reviewed Apr 9, 2026

View reviewed changes

Comment thread vortex-array/src/arrays/listview/compute/take.rs Outdated

dimitarvdimitrov added 4 commits April 9, 2026 17:58

Only sometimes rebuild

9694e13

Signed-off-by: Dimitar Dimitrov <dimitar@spiraldb.com>

fix: avoid private intra-doc link to REBUILD_DENSITY_THRESHOLD

6a7b4b9

Signed-off-by: Dimitar Dimitrov <dimitar@spiraldb.com>

Always only sometimes rebuild even when we know we have to rebuild

e0a30c5

Signed-off-by: Dimitar Dimitrov <dimitar@spiraldb.com>

Fix listview CI checks

a108dc6

Signed-off-by: Dimitar Dimitrov <dimitar@spiraldb.com>

dimitarvdimitrov requested a review from joseph-isaacs April 10, 2026 11:00

joseph-isaacs reviewed Apr 10, 2026

View reviewed changes

joseph-isaacs added the action/benchmark Trigger full benchmarks to run on this PR label Apr 10, 2026

dimitarvdimitrov added 2 commits April 10, 2026 15:36

pub(crate)

3baac20

Signed-off-by: Dimitar Dimitrov <dimitar@spiraldb.com>

Merge remote-tracking branch 'origin/develop' into mitko/avoid-list-v…

df8bb7d

…iew-take-reduce-rebuild

dimitarvdimitrov requested a review from joseph-isaacs April 16, 2026 14:13

joseph-isaacs reviewed Apr 16, 2026

View reviewed changes

joseph-isaacs approved these changes Apr 16, 2026

View reviewed changes

joseph-isaacs added action/benchmark Trigger full benchmarks to run on this PR and removed action/benchmark Trigger full benchmarks to run on this PR labels Apr 16, 2026

github-actions bot removed the action/benchmark Trigger full benchmarks to run on this PR label Apr 16, 2026

robert3005 added changelog/performance A performance improvement and removed changelog/skip Do not list PR in the changelog labels Apr 16, 2026

refactor: move listview rebuild density threshold to compute mod

2eb3db2

Signed-off-by: Dimitar Dimitrov <dimitar@spiraldb.com>

Conversation

dimitarvdimitrov commented Apr 8, 2026

Uh oh!

Uh oh!

Uh oh!

codspeed-hq bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will improve performance by 20.23%

Performance Changes

Footnotes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gatesn commented Apr 8, 2026

Uh oh!

dimitarvdimitrov commented Apr 8, 2026

Uh oh!

gatesn commented Apr 8, 2026

Uh oh!

dimitarvdimitrov commented Apr 9, 2026

Uh oh!

Uh oh!

joseph-isaacs Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

dimitarvdimitrov Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

joseph-isaacs Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

joseph-isaacs left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 16, 2026

BENCHMARK FAILED

Uh oh!

github-actions bot commented Apr 16, 2026

BENCHMARK FAILED

Uh oh!

github-actions bot commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Polar Signals Profiling Results

Latest Run

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codspeed-hq bot commented Apr 8, 2026 •

edited

Loading

github-actions bot commented Apr 16, 2026 •

edited

Loading